Skip to content

feat(sdk): initial obligations support in rewrap flow #748

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 36 commits into from
Oct 17, 2025
Merged

feat(sdk): initial obligations support in rewrap flow #748

merged 36 commits into from
Oct 17, 2025

Conversation

jakedoublev
Copy link
Contributor

@jakedoublev jakedoublev commented Oct 6, 2025 •
edited
Loading

  • send X-Rewrap-Additional-Context to KAS.Rewrap during decrypt from client config or reader config specified fulfillableObligationFQNs (with reader config obligations taking preference as most granular specificity)
  • access kas rewrap response responses > results > metadata for special obligations metadata key
  • merge/deduplicate obligations and surface them on TDF readers
  • provide reader with obligations() method that retrieves obligations through the rewrap flow

@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
jakedoublev and others added 2 commits October 7, 2025 10:10
@jakedoublev
🤖 🎨 Autoformat
3788ebf 
Signed-off-by: jakedoublev <[email protected]>
@jakedoublev
wip
2fe0892 
Signed-off-by: jakedoublev <[email protected]>
@jakedoublev jakedoublev force-pushed the feat/DSPX-1367-obligations branch from 2173ca8 to 2fe0892 Compare October 7, 2025 17:10
@jakedoublev jakedoublev changed the title feat(core): initial obligations support in rewrap flow feat(sdk): initial obligations support in rewrap flow Oct 8, 2025
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
@jakedoublev
🤖 🎨 Autoformat
2c4109d 
Signed-off-by: jakedoublev <[email protected]>
@jakedoublev jakedoublev force-pushed the feat/DSPX-1367-obligations branch from 20c4c05 to 45c7127 Compare October 8, 2025 19:45
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
@jakedoublev
lint fix
a17b4ba 
Signed-off-by: jakedoublev <[email protected]>
@jakedoublev
tests
d1480b9 
Signed-off-by: jakedoublev <[email protected]>
@jakedoublev jakedoublev force-pushed the feat/DSPX-1367-obligations branch from 7f6e8c0 to d1480b9 Compare October 8, 2025 22:34
@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@jakedoublev
🤖 🎨 Autoformat
b19ec8e 
Signed-off-by: jakedoublev <[email protected]>
@jakedoublev jakedoublev force-pushed the feat/DSPX-1367-obligations branch from 80ea30a to b19ec8e Compare October 8, 2025 22:37
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
@jakedoublev jakedoublev force-pushed the feat/DSPX-1367-obligations branch from 71084e3 to 8b3d2c0 Compare October 15, 2025 20:46
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
1 similar comment
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
@jakedoublev
🤖 🎨 Autoformat
ed3b4a5 
Signed-off-by: jakedoublev <[email protected]>
@jakedoublev jakedoublev force-pushed the feat/DSPX-1367-obligations branch from 7b91fdd to ed3b4a5 Compare October 17, 2025 03:40
@jakedoublev
Copy link
Contributor Author

3/4 of the Sonarcloud issues are existing code I only touched, and the 4th asking for DRYness within the ZTDFReader and NanoTDFReader I would rather avoid because they satisfy the same interface but may differ in implementation as Nano is aligned with other ZTDF features like splits and streaming.

c-r33d
c-r33d reviewed Oct 17, 2025
View reviewed changes
c-r33d
c-r33d reviewed Oct 17, 2025
View reviewed changes
c-r33d
c-r33d reviewed Oct 17, 2025
View reviewed changes
c-r33d
c-r33d reviewed Oct 17, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link

If these changes look good, signoff on them with:

git pull && git commit --amend --signoff && git push --force-with-lease origin

If they aren't any good, please remove them with:

git pull && git reset --hard HEAD~1 && git push --force-with-lease origin

jakedoublev and others added 2 commits October 17, 2025 07:34
@jakedoublev
pr feedback
db1d725 
Signed-off-by: jakedoublev <[email protected]>
@jakedoublev
🤖 🎨 Autoformat
bbebc87 
Signed-off-by: jakedoublev <[email protected]>
@jakedoublev jakedoublev force-pushed the feat/DSPX-1367-obligations branch from 98ca20b to bbebc87 Compare October 17, 2025 14:34
c-r33d
c-r33d previously approved these changes Oct 17, 2025
View reviewed changes
Copy link

@c-r33d c-r33d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to have Nick or Eugene review too, looks good to me.

eugenioenko
eugenioenko reviewed Oct 17, 2025
View reviewed changes
eugenioenko
eugenioenko reviewed Oct 17, 2025
View reviewed changes
eugenioenko
eugenioenko reviewed Oct 17, 2025
View reviewed changes
eugenioenko
eugenioenko previously approved these changes Oct 17, 2025
View reviewed changes
Copy link
Contributor

@eugenioenko eugenioenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, left small inline comments that can be addressed later on, non-blocking from merging

ntrevino-virtru
ntrevino-virtru previously approved these changes Oct 17, 2025
View reviewed changes
Copy link
Contributor

@ntrevino-virtru ntrevino-virtru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely seems fine, and like it will work. From an API standpoint, I'm not sure I'm fond of mixing setting this.requiredObligations in with a call to decrypt. I think it could always be cleaned up later though.

jakedoublev
jakedoublev commented Oct 17, 2025
View reviewed changes
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@jakedoublev jakedoublev merged commit 0361361 into main Oct 17, 2025
17 checks passed
@jakedoublev jakedoublev deleted the feat/DSPX-1367-obligations branch October 17, 2025 18:59
@opentdf-automation opentdf-automation bot mentioned this pull request Oct 17, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ntrevino-virtru ntrevino-virtru ntrevino-virtru approved these changes

@eugenioenko eugenioenko eugenioenko left review comments

@c-r33d c-r33d c-r33d left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jakedoublev @eugenioenko @ntrevino-virtru @c-r33d